Conversation
DorianZheng
reviewed
May 27, 2026
| /// default lean kernel. Only effective when the binary was built | ||
| /// with `--features kernel-net` (or both `kernel-lean,kernel-net`). | ||
| #[serde(default)] | ||
| pub kernel_net: bool, |
Member
There was a problem hiding this comment.
i think it would be make sense for user to specify kernel path themself?
G4614
force-pushed
the
feat/kernel-selection
branch
from
May 27, 2026 07:24
5598e40 to
83afe8c
Compare
G4614
marked this pull request as draft
May 27, 2026 11:58
G4614
force-pushed
the
feat/kernel-selection
branch
from
May 28, 2026 03:23
8af6fa2 to
6718a8c
Compare
G4614
added a commit
to G4614/boxlite
that referenced
this pull request
May 28, 2026
Addresses @DorianZheng's boxlite-ai#596 review: users should be able to compile their own kernel, not just pick the built-in lean/net presets. The load side already accepts arbitrary blobs (`--kernel <path>` → stage_custom_kernel symlinks + dlopens at runtime, no rebuild). This adds the build side: - Extract the net build into a generalized `scripts/build/build-libkrunfw.sh` parameterized by OVERLAY / KCONFIG / SONAME / OUT, with a DRY_RUN mode that validates the overlay + config merge without the ~10-20 min build. - `build-libkrunfw-net.sh` becomes a thin wrapper pinning the net overlay, the distinct `libkrunfw-net.so.5` SONAME, and the canonical embed path (behavior unchanged — DRY_RUN confirms identical resolved config/paths). - `make libkrunfw-custom OVERLAY=... [OUT=...]` builds a blob with the default `libkrunfw.so.5` SONAME; load it with `boxlite run --kernel <OUT>`. - README documents the custom workflow. Validated via DRY_RUN (net delegation + custom + missing-OVERLAY error); real kernel build not run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
G4614
added a commit
to G4614/boxlite
that referenced
this pull request
Jun 1, 2026
Addresses @DorianZheng's boxlite-ai#596 review: users should be able to compile their own kernel, not just pick the built-in lean/net presets. The load side already accepts arbitrary blobs (`--kernel <path>` → stage_custom_kernel symlinks + dlopens at runtime, no rebuild). This adds the build side: - Extract the net build into a generalized `scripts/build/build-libkrunfw.sh` parameterized by OVERLAY / KCONFIG / SONAME / OUT, with a DRY_RUN mode that validates the overlay + config merge without the ~10-20 min build. - `build-libkrunfw-net.sh` becomes a thin wrapper pinning the net overlay, the distinct `libkrunfw-net.so.5` SONAME, and the canonical embed path (behavior unchanged — DRY_RUN confirms identical resolved config/paths). - `make libkrunfw-custom OVERLAY=... [OUT=...]` builds a blob with the default `libkrunfw.so.5` SONAME; load it with `boxlite run --kernel <OUT>`. - README documents the custom workflow. Validated via DRY_RUN (net delegation + custom + missing-OVERLAY error); real kernel build not run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
G4614
force-pushed
the
feat/kernel-selection
branch
from
June 1, 2026 04:22
1e94a09 to
2756b81
Compare
5 of 6 tasks
G4614
added a commit
to G4614/boxlite
that referenced
this pull request
Jun 2, 2026
Addresses @DorianZheng's boxlite-ai#596 review: users should be able to compile their own kernel, not just pick the built-in lean/net presets. The load side already accepts arbitrary blobs (`--kernel <path>` → stage_custom_kernel symlinks + dlopens at runtime, no rebuild). This adds the build side: - Extract the net build into a generalized `scripts/build/build-libkrunfw.sh` parameterized by OVERLAY / KCONFIG / SONAME / OUT, with a DRY_RUN mode that validates the overlay + config merge without the ~10-20 min build. - `build-libkrunfw-net.sh` becomes a thin wrapper pinning the net overlay, the distinct `libkrunfw-net.so.5` SONAME, and the canonical embed path (behavior unchanged — DRY_RUN confirms identical resolved config/paths). - `make libkrunfw-custom OVERLAY=... [OUT=...]` builds a blob with the default `libkrunfw.so.5` SONAME; load it with `boxlite run --kernel <OUT>`. - README documents the custom workflow. Validated via DRY_RUN (net delegation + custom + missing-OVERLAY error); real kernel build not run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
G4614
force-pushed
the
feat/kernel-selection
branch
from
June 2, 2026 10:03
2756b81 to
6f87201
Compare
G4614
added a commit
to G4614/boxlite
that referenced
this pull request
Jul 14, 2026
Addresses @DorianZheng's boxlite-ai#596 review: users should be able to compile their own kernel, not just pick the built-in lean/net presets. The load side already accepts arbitrary blobs (`--kernel <path>` → stage_custom_kernel symlinks + dlopens at runtime, no rebuild). This adds the build side: - Extract the net build into a generalized `scripts/build/build-libkrunfw.sh` parameterized by OVERLAY / KCONFIG / SONAME / OUT, with a DRY_RUN mode that validates the overlay + config merge without the ~10-20 min build. - `build-libkrunfw-net.sh` becomes a thin wrapper pinning the net overlay, the distinct `libkrunfw-net.so.5` SONAME, and the canonical embed path (behavior unchanged — DRY_RUN confirms identical resolved config/paths). - `make libkrunfw-custom OVERLAY=... [OUT=...]` builds a blob with the default `libkrunfw.so.5` SONAME; load it with `boxlite run --kernel <OUT>`. - README documents the custom workflow. Validated via DRY_RUN (net delegation + custom + missing-OVERLAY error); real kernel build not run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
G4614
force-pushed
the
feat/kernel-selection
branch
from
July 14, 2026 08:28
6f87201 to
df59d59
Compare
Contributor
📝 WalkthroughWalkthroughAdds lean and net embedded kernel variants, including net-specific kernel builds, feature-based artifact packaging, runtime selection, CLI support, and iptables integration tests. Development dependency setup is made conditional, and detached-box cleanup infrastructure is added for tests. ChangesKernel Variant Support
Test and Development Tooling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Spawner
participant KernelLibrary
User->>CLI: pass --kernel-variant net
CLI->>Spawner: set BoxOptions.kernel_variant
Spawner->>KernelLibrary: stage libkrunfw-net.so.5
Spawner->>Spawner: prepend staged library path
Spawner-->>CLI: launch box with net kernel
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
G4614
added a commit
to G4614/boxlite
that referenced
this pull request
Jul 23, 2026
Addresses @DorianZheng's boxlite-ai#596 review: users should be able to compile their own kernel, not just pick the built-in lean/net presets. The load side already accepts arbitrary blobs (`--kernel <path>` → stage_custom_kernel symlinks + dlopens at runtime, no rebuild). This adds the build side: - Extract the net build into a generalized `scripts/build/build-libkrunfw.sh` parameterized by OVERLAY / KCONFIG / SONAME / OUT, with a DRY_RUN mode that validates the overlay + config merge without the ~10-20 min build. - `build-libkrunfw-net.sh` becomes a thin wrapper pinning the net overlay, the distinct `libkrunfw-net.so.5` SONAME, and the canonical embed path (behavior unchanged — DRY_RUN confirms identical resolved config/paths). - `make libkrunfw-custom OVERLAY=... [OUT=...]` builds a blob with the default `libkrunfw.so.5` SONAME; load it with `boxlite run --kernel <OUT>`. - README documents the custom workflow. Validated via DRY_RUN (net delegation + custom + missing-OVERLAY error); real kernel build not run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
G4614
force-pushed
the
feat/kernel-selection
branch
from
July 23, 2026 10:15
df59d59 to
a07c1f9
Compare
RAII guard that SIGKILLs detached boxes on Drop. Scans /proc/*/fd for FDs referencing the box's working directory — the only reliable fingerprint after the shim daemonizes and removes its PID file. Runs on panic too, preventing test leakage of libkrun VMs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Kernel blob selection is now split across two layers: **Build time** (cargo features): cargo build → lean only (default) cargo build --features kernel-net → net only cargo build --features kernel-lean,kernel-net → both (dual mode) **Runtime** (CLI flag, only meaningful in dual mode): boxlite run alpine → uses default (lean) kernel boxlite run --kernel net alpine → uses net kernel Single-kernel builds ignore --kernel; mismatch (e.g. --kernel net on a lean-only build) produces a clear error pointing to the missing feature flag. The net kernel adds ~50 modules (netfilter/nf_tables/bridge/NET_NS) on top of the lean kernel. Required by dockerd/dind workloads that need iptables and bridge networking inside the VM. Build infra: kconfig overlays, build-libkrunfw-net.sh, auto-download from GitHub releases (same pipeline as lean kernel). Developers can override with BOXLITE_LIBKRUNFW_NET_PATH for locally built blobs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kernel_net_has_iptables tried to skip when binary lacks --features kernel-net, but checked stdout while the dependency error surfaces via tracing on stderr — skip never triggered, test hard-failed on default lean builds. Capture both streams; check either for the dependency requirement string. Default build now skips correctly; --features kernel-net build still exercises the assertion. Also fix sdks/node/src/options.rs: BoxOptions gained a kernel field in this PR but the Node SDK's JS-to-Rust conversion still built BoxOptions without it (clippy E0063). Add kernel: None there (Node SDK doesn't yet expose --kernel; runtime defaults to lean).
Addresses @DorianZheng's boxlite-ai#596 review: users should be able to compile their own kernel, not just pick the built-in lean/net presets. The load side already accepts arbitrary blobs (`--kernel <path>` → stage_custom_kernel symlinks + dlopens at runtime, no rebuild). This adds the build side: - Extract the net build into a generalized `scripts/build/build-libkrunfw.sh` parameterized by OVERLAY / KCONFIG / SONAME / OUT, with a DRY_RUN mode that validates the overlay + config merge without the ~10-20 min build. - `build-libkrunfw-net.sh` becomes a thin wrapper pinning the net overlay, the distinct `libkrunfw-net.so.5` SONAME, and the canonical embed path (behavior unchanged — DRY_RUN confirms identical resolved config/paths). - `make libkrunfw-custom OVERLAY=... [OUT=...]` builds a blob with the default `libkrunfw.so.5` SONAME; load it with `boxlite run --kernel <OUT>`. - README documents the custom workflow. Validated via DRY_RUN (net delegation + custom + missing-OVERLAY error); real kernel build not run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The path arm already worked (apply_to's `_` => opts.kernel = Some(k)), but the flag help only mentioned `net`, so the public custom-kernel capability was undiscoverable. Update the doc + value_name to `net|PATH` and point at `make libkrunfw-custom`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
G4614
force-pushed
the
feat/kernel-selection
branch
from
July 27, 2026 16:02
c6b441b to
049ff38
Compare
G4614
changed the title
Contributor
Author
Contributor
Author
G4614
marked this pull request as ready for review
July 28, 2026 07:11
📦 BoxLite review — 2 issues ·
|
![boxlite-agent[bot]](https://avatars.githubusercontent.com/in/4244458?s=60&v=4){kind=small}
Comment on lines
+38
to
+42
| const LIBKRUNFW_NET_URL: &str = | ||
| "https://github.com/G4614/boxlite/releases/download/v0.9.5-kernel-net/libkrunfw-net-x86_64.tgz"; | ||
| #[cfg(all(target_os = "linux", target_arch = "x86_64"))] | ||
| const LIBKRUNFW_NET_SHA256: &str = | ||
| "f367a6e96ba7f4d11d1837b871c91308d6025ce8dbecee4e5fc914aacf28f128"; |
There was a problem hiding this comment.
🛑 Net kernel fetched from personal GitHub fork
cargo build --features kernel-net on Linux/x86_64 downloads and embeds a privileged libkrunfw blob from https://github.com/G4614/boxlite (not boxlite-ai org); SHA256 pin only stops corruption/tampering-in-transit, not the fork owner shipping a bad blob at that URL — confirmed by reading build.rs:38-42, feature is opt-in but shipped with a working (non-TODO) hash so it's not blocked from being built today.
Comment on lines
+45
to
+48
| const LIBKRUNFW_NET_URL: &str = | ||
| "https://github.com/boxlite-ai/boxlite/releases/download/v0.9.5/libkrunfw-net-aarch64.tgz"; | ||
| #[cfg(all(target_os = "linux", target_arch = "aarch64"))] | ||
| const LIBKRUNFW_NET_SHA256: &str = "TODO_FILL_AFTER_UPLOAD"; |
There was a problem hiding this comment.
LIBKRUNFW_NET_SHA256 for aarch64 is the placeholder string "TODO_FILL_AFTER_UPLOAD"; guarded by a panic in download_libkrunfw_so so it fails loudly rather than silently, but --features kernel-net is non-functional on aarch64 until filled in.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (1)
src/test-utils/src/box_cleanup.rs (1)
32-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMark
BoxCleanupas#[must_use].Because cleanup is destructive and deferred through
Drop, a temporary can terminate the VM immediately instead of remaining alive for the intended scope. A#[must_use]annotation would catch this misuse at compile time.Suggested change
+#[must_use = "bind BoxCleanup to defer cleanup until scope exit"] pub struct BoxCleanup {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test-utils/src/box_cleanup.rs` around lines 32 - 35, Annotate the BoxCleanup struct with #[must_use] so callers receive a compiler warning when the deferred cleanup guard is created and immediately discarded. Keep the existing fields and Drop behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@make/build.mk`:
- Around line 33-53: Update the net-kernel documentation in make/build.mk lines
33-53 to use the exposed --kernel-variant net option and
BOXLITE_LIBKRUNFW_NET_PATH variable instead of outdated names; also update
src/deps/libkrun-sys/net-configs/overlay-net_x86_64 lines 1-6, replacing make
libkrunfw-dind with make libkrunfw-net.
In `@make/dev.mk`:
- Around line 9-10: The development setup must resolve tools from the activated
virtual environment rather than PATH. In make/dev.mk lines 9-10, update the uv
fallback check/install and dependency-group install to use $VIRTUAL_ENV/bin/uv;
in make/dev.mk lines 35-36, update the maturin fallback and develop command to
use $VIRTUAL_ENV/bin/maturin.
In `@scripts/build/build-libkrunfw.sh`:
- Around line 102-110: Update the artifact-copy logic in the build script to
resolve and copy the target selected by the libkrunfw.so.5 SONAME symlink,
dereferencing the symlink rather than choosing the lexicographically first
libkrunfw.so.5.* file. Preserve the existing missing-artifact failure behavior
while ensuring the output contains the current build’s selected library.
In `@src/boxlite/src/util/mod.rs`:
- Around line 162-165: Update the library-directory construction around
EmbeddedRuntime::get to honor the BOXLITE_RUNTIME_DIR override before falling
back to the embedded runtime directory. Preserve the existing prepend behavior
so the explicitly supplied runtime path is forwarded to the shim and included in
its loader path.
In `@src/boxlite/src/vmm/controller/spawn.rs`:
- Around line 218-220: Update the net firmware lookup near net_blob to select
libkrunfw.so.5 for net-only kernel-feature builds and libkrunfw-net.so.5 for
dual-mode builds. Preserve the existing missing-blob handling after resolving
the feature-dependent filename.
In `@src/deps/libkrun-sys/build.rs`:
- Around line 35-48: The aarch64 net firmware release is incomplete and the
build must not silently fall back to lean-only firmware. In
src/deps/libkrun-sys/build.rs lines 35-48, publish the aarch64 artifact under a
controlled release and replace LIBKRUNFW_NET_SHA256’s placeholder with the
verified SHA256; in lines 279-294, update the net-firmware selection flow to
return a build error when no verified remote artifact or local override is
available.
- Around line 297-304: Update the net-kernel setup flow around
LIBKRUNFW_NET_SHA256 and local_net so a configured BOXLITE_LIBKRUNFW_NET_PATH
override is handled first via install_local_net_kernel, without requiring a
non-placeholder checksum. Only panic for the missing checksum when no local net
artifact is configured.
In `@src/deps/libkrun-sys/net-configs/overlay-net_aarch64`:
- Line 1: Correct the header comment in the aarch64 overlay configuration to
identify the aarch64 base configuration and the libkrunfw-net build target. Keep
the change limited to the comment so it accurately reflects how this file is
selected and applied.
In `@src/test-utils/src/box_cleanup.rs`:
- Around line 39-40: Update the cleanup path matching around the needle and
home_prefix construction to use normalized Path values: build the complete box
path with home_path.join("boxes").join(&box_id), then use component-aware
Path::starts_with checks instead of string prefix matching. Preserve the
existing process cleanup behavior while ensuring only processes under the exact
box directory are matched.
- Around line 42-68: Update the cleanup logic around the `/proc` scan and
`Command::new("kill")` so failures from `read_dir`, `read_link`, and command
execution are recorded and surfaced rather than silently ignored. Only push a
PID into `killed` when the kill command completes successfully with a successful
exit status; preserve the existing matching behavior while ensuring the final
cleanup report distinguishes failures from successful termination.
- Around line 42-64: Update the process cleanup loop around the `/proc` scan and
`matched` check to preserve process identity between inspection and termination,
rather than invoking `kill -9` with the reused numeric PID. Open and retain an
identity-preserving handle such as a pidfd before scanning each process, then
signal through that handle only when `matched` is true; keep recording the
original PID in `killed`.
---
Nitpick comments:
In `@src/test-utils/src/box_cleanup.rs`:
- Around line 32-35: Annotate the BoxCleanup struct with #[must_use] so callers
receive a compiler warning when the deferred cleanup guard is created and
immediately discarded. Keep the existing fields and Drop behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5aec56b4-9128-41cf-8f93-9b33b4d8329e
📒 Files selected for processing (20)
make/build.mkmake/dev.mkscripts/build/build-libkrunfw-net.shscripts/build/build-libkrunfw.shsdks/node/src/options.rssrc/boxlite/Cargo.tomlsrc/boxlite/src/runtime/options.rssrc/boxlite/src/util/mod.rssrc/boxlite/src/vmm/controller/spawn.rssrc/cli/Cargo.tomlsrc/cli/README.mdsrc/cli/src/cli.rssrc/cli/tests/kernel_net_iptables.rssrc/deps/libkrun-sys/Cargo.tomlsrc/deps/libkrun-sys/build.rssrc/deps/libkrun-sys/net-configs/README.mdsrc/deps/libkrun-sys/net-configs/overlay-net_aarch64src/deps/libkrun-sys/net-configs/overlay-net_x86_64src/test-utils/src/box_cleanup.rssrc/test-utils/src/lib.rs
Comment on lines
+33
to
+53
| # Build the "fat" libkrunfw variant required by `boxlite run --net-kernel` | ||
| # (issue #276): the lean default kernel lacks CONFIG_BRIDGE/NETFILTER/NF_NAT/ | ||
| # IPTABLE_*/NF_TABLES, which docker / docker-compose need for bridge networks, | ||
| # NAT and iptables rule installation. This target builds a second libkrunfw | ||
| # blob with those configs added on top of the lean config, and copies it to | ||
| # | ||
| # target/net-kernel/lib64/libkrunfw-net.so.5 | ||
| # | ||
| # Wire-up: the libkrun-sys build.rs auto-detects this blob at the canonical | ||
| # path above on the next cargo build — no env var required. (Set | ||
| # BOXLITE_LIBKRUNFW_PRIVILEGED_PATH only when the blob lives outside the | ||
| # workspace, e.g., a CI cache or sysroot.) Without this target ever being run, | ||
| # `--net-kernel` still applies the userspace changes (cgroup rw + full caps) | ||
| # but the kernel stays lean, so bridge / iptables-dependent features keep | ||
| # failing. With it run, the net-kernel blob is staged alongside the lean one | ||
| # and the runtime picks the right blob per-box. | ||
| # | ||
| # Heavy target (~10–20 min, downloads kernel source). Only run when actively | ||
| # iterating on the net-kernel kernel feature; not in any other target's dep chain. | ||
| libkrunfw-net: | ||
| @bash $(SCRIPT_DIR)/build/build-libkrunfw-net.sh |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align net-kernel documentation with the exposed interfaces. Both locations retain pre-rename commands or variables, so users following the build instructions will use unsupported names.
make/build.mk#L33-L53: document--kernel-variant netandBOXLITE_LIBKRUNFW_NET_PATH.src/deps/libkrun-sys/net-configs/overlay-net_x86_64#L1-L6: replacemake libkrunfw-dindwithmake libkrunfw-net.
📍 Affects 2 files
make/build.mk#L33-L53(this comment)src/deps/libkrun-sys/net-configs/overlay-net_x86_64#L1-L6
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@make/build.mk` around lines 33 - 53, Update the net-kernel documentation in
make/build.mk lines 33-53 to use the exposed --kernel-variant net option and
BOXLITE_LIBKRUNFW_NET_PATH variable instead of outdated names; also update
src/deps/libkrun-sys/net-configs/overlay-net_x86_64 lines 1-6, replacing make
libkrunfw-dind with make libkrunfw-net.
Comment on lines
+9
to
+10
| @. .venv/bin/activate && { command -v uv >/dev/null || pip install -q uv; } && \ | ||
| (cd sdks/python && uv pip install --group dev --group sync) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
. .venv/bin/activate
echo "uv: $(command -v uv || true)"
uv --version
uv pip install --help | grep -F -- '--group'
echo "maturin: $(command -v maturin || true)"
maturin --version
maturin develop --help | grep -F -- '--uv'Repository: boxlite-ai/boxlite
Length of output: 216
🏁 Script executed:
#!/bin/bash
set -u
printf '--- make/dev.mk context ---\n'
if [ -f make/dev.mk ]; then
cat -n make/dev.mk | sed -n '1,60p'
else
echo 'make/dev.mk not found'
fi
printf '\n--- repository files matching activators or tool installs ---\n'
git ls-files | rg '(^|/)(Makefile|dev\.mk|pyproject\.toml|uv\.lock|Cargo\.toml|\.python-version|requirements|Pipfile|poetry\.lock|taskfile|devcontainer|Dockerfile)' || true
printf '\n--- usages of command -v uv/maturin in tracked files ---\n'
rg -n '--command -v (uv|maturin)|command -v (uv|maturin)|VIRTUAL_ENV|/bin/(uv|maturin)' . || trueRepository: boxlite-ai/boxlite
Length of output: 4195
🌐 Web query:
uv dependency groups uv pip install --group syntax version introduced
💡 Result:
Support for the [dependency-groups] table (PEP 735) and the associated --group syntax was introduced in uv version 0.4.27, released on October 25, 2024 [1][2][3]. While the --group flag was initially introduced in 0.4.27 as part of the core project management functionality (e.g., for use with uv add, uv sync, etc.) [2], support for the --group flag within the uv pip install and uv pip compile commands was added later, in uv version 0.6.0 (via PR #11686, which merged in early 2025) [4][5]. The --group syntax generally follows the format --group <[path:]name>, where the path is optional and defaults to the pyproject.toml file in the current directory [4][6]. For example, you can use uv pip install --group mygroup to install a specific dependency group defined in your project's pyproject.toml [7][4].
Citations:
- 1: Support for PEP 735: Dependency Groups in
pyproject.tomlastral-sh/uv#8090 - 2: https://github.com/astral-sh/uv/releases/tag/0.4.27
- 3: https://newreleases.io/project/pypi/uv/release/0.4.27
- 4: astral-sh/uv@ba73231
- 5: Add dependency-group cli flags to
uv pip installanduv pip compile(--group,--no-group,--only-group,--all-groups) astral-sh/uv#10861 - 6: https://pip.pypa.io/en/stable/cli/pip_install.html
- 7: https://github.com/astral-sh/uv/blob/93fc6f04/docs/pip/packages.md
🌐 Web query:
maturin develop --uv option version
💡 Result:
The --uv option in maturin develop was introduced in version 1.6.0 [1][2]. It allows the maturin develop command to use uv (specifically the uv pip install interface) for package installation instead of the default pip [3][4][2]. Key details regarding this option include: - Purpose: It provides a faster, modern alternative to pip for installing your project during development [1][2]. - Mechanism: Maturin does not perform dependency resolution itself; when --uv is used, it builds the Rust extension and then executes uv pip install with the relevant requirements [5]. - Version Requirement: Users must be on maturin version 1.6.0 or higher to access this flag [2]. - Configuration: While you can pass the flag via the command line (maturin develop --uv) [3][4], some users integrate it into their development workflows by using it alongside uv-managed virtual environments or configuration files [5][6]. If you encounter unexpected argument errors, ensure your maturin version is at least 1.6.0 [2]. If you are already on a supported version but still face issues, confirm that uv is available in your system's PATH, as maturin calls the uv CLI as a subprocess [1][2].
Citations:
- 1: Add uv as develop backend command PyO3/maturin#2015
- 2: option to use uv PyO3/maturin#1959
- 3: https://www.maturin.rs/local_development.html
- 4: https://www.maturin.rs/local_development
- 5: Using Maturin with a local python dependency. PyO3/maturin#2774
- 6: Make uv and maturin work well together PyO3/maturin#2314
Ensure the tools come from the virtual environment.
command -v uv / command -v maturin can detect older global tools and skip the fallback install, while these commands require modern supported versions (uv pip install --group and maturin develop --uv). Resolve them from the activated venv before running them:
make/dev.mk#L9-L10: use$VIRTUAL_ENV/bin/uvfor both the fallback install and the dependency-group install.make/dev.mk#L35-L36: use$VIRTUAL_ENV/bin/maturinfor the fallback install and formaturin develop --uv.
📍 Affects 1 file
make/dev.mk#L9-L10(this comment)make/dev.mk#L35-L36
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@make/dev.mk` around lines 9 - 10, The development setup must resolve tools
from the activated virtual environment rather than PATH. In make/dev.mk lines
9-10, update the uv fallback check/install and dependency-group install to use
$VIRTUAL_ENV/bin/uv; in make/dev.mk lines 35-36, update the maturin fallback and
develop command to use $VIRTUAL_ENV/bin/maturin.
Source: MCP tools
Comment on lines
+102
to
+110
| # libkrunfw's Makefile produces libkrunfw.so.5.<minor>.<patch> with a symlink | ||
| # chain libkrunfw.so.5 → it. Copy the real file and stamp the requested SONAME. | ||
| REAL_BLOB=$(ls "$LIBKRUNFW_SRC"/libkrunfw.so.5.* 2>/dev/null | head -1 || true) | ||
| if [ -z "$REAL_BLOB" ]; then | ||
| echo "❌ build succeeded but couldn't find libkrunfw.so.5.* in $LIBKRUNFW_SRC" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| cp "$REAL_BLOB" "$OUT" |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Copy the artifact selected by the SONAME symlink.
Line 104 picks the lexicographically first historical blob. If multiple versions remain, this can embed an old kernel despite a successful build. Copy libkrunfw.so.5 with symlink dereferencing instead.
Proposed fix
-REAL_BLOB=$(ls "$LIBKRUNFW_SRC"/libkrunfw.so.5.* 2>/dev/null | head -1 || true)
+REAL_BLOB="$LIBKRUNFW_SRC/libkrunfw.so.5"
if [ -z "$REAL_BLOB" ]; then
echo "❌ build succeeded but couldn't find libkrunfw.so.5.* in $LIBKRUNFW_SRC" >&2
exit 1
fi
-cp "$REAL_BLOB" "$OUT"
+cp -L "$REAL_BLOB" "$OUT"🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 104-104: Use find instead of ls to better handle non-alphanumeric filenames.
(SC2012)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/build/build-libkrunfw.sh` around lines 102 - 110, Update the
artifact-copy logic in the build script to resolve and copy the target selected
by the libkrunfw.so.5 SONAME symlink, dereferencing the symlink rather than
choosing the lexicographically first libkrunfw.so.5.* file. Preserve the
existing missing-artifact failure behavior while ensuring the output contains
the current build’s selected library.
Comment on lines
+162
to
+165
| #[cfg(feature = "embedded-runtime")] | ||
| if let Some(runtime) = crate::runtime::embedded::EmbeddedRuntime::get() { | ||
| lib_dirs.push(runtime.dir().to_path_buf()); | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Preserve BOXLITE_RUNTIME_DIR in the prepend variant.
Unlike configure_library_env, this path ignores the explicit runtime override. Since spawning now always uses this function, externally supplied runtime libraries are no longer forwarded to the shim or added to its loader path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/boxlite/src/util/mod.rs` around lines 162 - 165, Update the
library-directory construction around EmbeddedRuntime::get to honor the
BOXLITE_RUNTIME_DIR override before falling back to the embedded runtime
directory. Preserve the existing prepend behavior so the explicitly supplied
runtime path is forwarded to the shim and included in its loader path.
Comment on lines
+218
to
+220
| let net_blob = runtime_dir.join("libkrunfw-net.so.5"); | ||
| if !net_blob.exists() { | ||
| return Ok(None); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Handle the net-only artifact name.
Net-only packaging installs the net firmware as libkrunfw.so.5, while dual mode uses libkrunfw-net.so.5. This unconditional secondary-name lookup makes documented net-only builds fail with the missing-blob error. Select the source filename from the compiled kernel-feature layout, while retaining the secondary filename for dual mode. citeturn0search1
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/boxlite/src/vmm/controller/spawn.rs` around lines 218 - 220, Update the
net firmware lookup near net_blob to select libkrunfw.so.5 for net-only
kernel-feature builds and libkrunfw-net.so.5 for dual-mode builds. Preserve the
existing missing-blob handling after resolving the feature-dependent filename.
Comment on lines
+297
to
+304
| if LIBKRUNFW_NET_SHA256.starts_with("TODO") { | ||
| panic!( | ||
| "kernel-net feature enabled but LIBKRUNFW_NET_SHA256 not configured. \ | ||
| Upload the net kernel blob and fill in the SHA256." | ||
| ); | ||
| } | ||
| if let Some(source) = &local_net { | ||
| install_local_net_kernel(source, &lib_dir, false); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Check the local override before rejecting the missing checksum.
Net-only builds panic on the placeholder checksum before reaching BOXLITE_LIBKRUNFW_NET_PATH, so an aarch64 local net artifact cannot be used.
Proposed fix
- if LIBKRUNFW_NET_SHA256.starts_with("TODO") {
- panic!(
- "kernel-net feature enabled but LIBKRUNFW_NET_SHA256 not configured. \
- Upload the net kernel blob and fill in the SHA256."
- );
- }
if let Some(source) = &local_net {
install_local_net_kernel(source, &lib_dir, false);
} else {
+ if LIBKRUNFW_NET_SHA256.starts_with("TODO") {
+ panic!(
+ "kernel-net feature enabled but LIBKRUNFW_NET_SHA256 not configured. \
+ Upload the net kernel blob or set BOXLITE_LIBKRUNFW_NET_PATH."
+ );
+ }
let tarball = install_dir.join(format!("libkrunfw-net-{LIBKRUNFW_VERSION}.tgz"));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if LIBKRUNFW_NET_SHA256.starts_with("TODO") { | |
| panic!( | |
| "kernel-net feature enabled but LIBKRUNFW_NET_SHA256 not configured. \ | |
| Upload the net kernel blob and fill in the SHA256." | |
| ); | |
| } | |
| if let Some(source) = &local_net { | |
| install_local_net_kernel(source, &lib_dir, false); | |
| if let Some(source) = &local_net { | |
| install_local_net_kernel(source, &lib_dir, false); | |
| } else { | |
| if LIBKRUNFW_NET_SHA256.starts_with("TODO") { | |
| panic!( | |
| "kernel-net feature enabled but LIBKRUNFW_NET_SHA256 not configured. \ | |
| Upload the net kernel blob or set BOXLITE_LIBKRUNFW_NET_PATH." | |
| ); | |
| } | |
| let tarball = install_dir.join(format!("libkrunfw-net-{LIBKRUNFW_VERSION}.tgz")); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/deps/libkrun-sys/build.rs` around lines 297 - 304, Update the net-kernel
setup flow around LIBKRUNFW_NET_SHA256 and local_net so a configured
BOXLITE_LIBKRUNFW_NET_PATH override is handled first via
install_local_net_kernel, without requiring a non-placeholder checksum. Only
panic for the missing checksum when no local net artifact is configured.
| @@ -0,0 +1,116 @@ | |||
| # Overlay applied on top of config-libkrunfw_x86_64 by `make libkrunfw-dind`. | |||
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the aarch64 overlay header.
This says config-libkrunfw_x86_64 and make libkrunfw-dind, but this file is selected for aarch64 by make libkrunfw-net. This misleads manual debugging. citeturn0search0
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/deps/libkrun-sys/net-configs/overlay-net_aarch64` at line 1, Correct the
header comment in the aarch64 overlay configuration to identify the aarch64 base
configuration and the libkrunfw-net build target. Keep the change limited to the
comment so it accurately reflects how this file is selected and applied.
Comment on lines
+39
to
+40
| let needle = format!("/boxes/{}/", self.box_id); | ||
| let home_prefix = self.home_path.to_string_lossy().into_owned(); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Match the complete box path with Path components.
The string checks can match sibling home directories or nested boxes paths, causing cleanup to SIGKILL an unrelated process. Build home_path.join("boxes").join(&box_id) and use component-aware Path::starts_with after normalizing the home path.
Suggested change
- let needle = format!("/boxes/{}/", self.box_id);
- let home_prefix = self.home_path.to_string_lossy().into_owned();
+ let box_prefix = self.home_path.join("boxes").join(&self.box_id);
...
- let s = tgt.to_string_lossy();
- s.starts_with(&home_prefix) && s.contains(&needle)
+ tgt.starts_with(&box_prefix)Also applies to: 54-59
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test-utils/src/box_cleanup.rs` around lines 39 - 40, Update the cleanup
path matching around the needle and home_prefix construction to use normalized
Path values: build the complete box path with
home_path.join("boxes").join(&box_id), then use component-aware
Path::starts_with checks instead of string prefix matching. Preserve the
existing process cleanup behavior while ensuring only processes under the exact
box directory are matched.
Comment on lines
+42
to
+64
| if let Ok(procs) = std::fs::read_dir("/proc") { | ||
| for proc_entry in procs.flatten() { | ||
| let Some(name) = proc_entry.file_name().to_str().map(str::to_owned) else { | ||
| continue; | ||
| }; | ||
| let Ok(pid) = name.parse::<u32>() else { | ||
| continue; | ||
| }; | ||
| let fd_dir = proc_entry.path().join("fd"); | ||
| let Ok(fds) = std::fs::read_dir(&fd_dir) else { | ||
| continue; | ||
| }; | ||
| let matched = fds.flatten().any(|fd| { | ||
| std::fs::read_link(fd.path()) | ||
| .map(|tgt| { | ||
| let s = tgt.to_string_lossy(); | ||
| s.starts_with(&home_prefix) && s.contains(&needle) | ||
| }) | ||
| .unwrap_or(false) | ||
| }); | ||
| if matched { | ||
| let _ = Command::new("kill").args(["-9", &pid.to_string()]).output(); | ||
| killed.push(pid); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg '(^|/)box_cleanup\.rs$|test-utils|src/test-utils' || true
echo "== file excerpt =="
if [ -f src/test-utils/src/box_cleanup.rs ]; then
nl -ba src/test-utils/src/box_cleanup.rs | sed -n '1,130p'
fi
echo "== search cleanup/tooling usage and rust pidfd support =="
rg -n "box_cleanup|pid_cleanup|kill|pidfd|/proc|SIGKILL|Box|process" src test src/test-utils Cargo.toml Cargo.lock 2>/dev/null | head -200 || true
echo "== rust toolchain/feature hints =="
fd -a "Cargo.toml|config.*" . | xargs -r rg -n "edition|msrv|rust-version|linux|pidfd|std::os::linux|process" 2>/dev/null | head -200 || trueRepository: boxlite-ai/boxlite
Length of output: 545
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
if [ -f src/test-utils/src/box_cleanup.rs ]; then
cat -n src/test-utils/src/box_cleanup.rs | sed -n '1,130p'
fi
echo "== usages =="
rg -n "box_cleanup|BoxCleanup|cleanup|kill|read_dir\\(\"/proc\"|pidfd" . 2>/dev/null | head -200 || true
echo "== package toolchain hints =="
for f in Cargo.toml src/test-utils/Cargo.toml; do
[ -f "$f" ] && { echo "--- $f"; cat -n "$f" | sed -n '1,220p'; }
done
echo "== deterministic TOCTOU semantic notes via source shape =="
python3 - <<'PY'
from pathlib import Path
p=Path("src/test-utils/src/box_cleanup.rs")
s=p.read_text() if p.exists() else ""
checks = {
"reads_numeric_pid": 'let Some(name) = proc_entry.file_name().to_str().map(str::to_owned)' in s,
"parses_u32": 'let Ok(pid) = name.parse::<u32>()' in s,
"calls_kill_later": '.args(["-9", &pid.to_string()])' in s,
"pidfd_present": 'pidfd' in s,
"signal_via_process_handle": 'kill()' in s and '/proc' not in s,
"separate_fs_before_kill": 'std::fs::read_link' in s and s.index('/proc') < s.index('.args(["-9", &pid.to_string()])'),
}
for k,v in checks.items():
print(f"{k}={v}")
PYRepository: boxlite-ai/boxlite
Length of output: 24143
Avoid the PID-reuse TOCTOU before sending SIGKILL.
This scan identifies <pid> from /proc, reads file descriptors, and only then kills the numeric PID. If the matched process exits and the kernel reuses that PID before cleanup runs, kill -9 <pid> can terminate an unrelated process. Maintain process identity across the inspect-kill boundary, e.g. open a pidfd before scanning and signal through it, or use another identity-preserving mechanism.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test-utils/src/box_cleanup.rs` around lines 42 - 64, Update the process
cleanup loop around the `/proc` scan and `matched` check to preserve process
identity between inspection and termination, rather than invoking `kill -9` with
the reused numeric PID. Open and retain an identity-preserving handle such as a
pidfd before scanning each process, then signal through that handle only when
`matched` is true; keep recording the original PID in `killed`.
Comment on lines
+42
to
+68
| if let Ok(procs) = std::fs::read_dir("/proc") { | ||
| for proc_entry in procs.flatten() { | ||
| let Some(name) = proc_entry.file_name().to_str().map(str::to_owned) else { | ||
| continue; | ||
| }; | ||
| let Ok(pid) = name.parse::<u32>() else { | ||
| continue; | ||
| }; | ||
| let fd_dir = proc_entry.path().join("fd"); | ||
| let Ok(fds) = std::fs::read_dir(&fd_dir) else { | ||
| continue; | ||
| }; | ||
| let matched = fds.flatten().any(|fd| { | ||
| std::fs::read_link(fd.path()) | ||
| .map(|tgt| { | ||
| let s = tgt.to_string_lossy(); | ||
| s.starts_with(&home_prefix) && s.contains(&needle) | ||
| }) | ||
| .unwrap_or(false) | ||
| }); | ||
| if matched { | ||
| let _ = Command::new("kill").args(["-9", &pid.to_string()]).output(); | ||
| killed.push(pid); | ||
| } | ||
| } | ||
| } | ||
| eprintln!("[cleanup] box {} SIGKILL'd pids={:?}", self.box_id, killed); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Surface cleanup failures instead of reporting false success.
read_dir, read_link, and Command::output failures are ignored, while killed.push(pid) runs unconditionally. Permission-restricted /proc, a missing kill utility, or a failed signal can therefore leave the VM alive while the log implies cleanup completed. Record failures and only add PIDs after a successful termination status.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test-utils/src/box_cleanup.rs` around lines 42 - 68, Update the cleanup
logic around the `/proc` scan and `Command::new("kill")` so failures from
`read_dir`, `read_link`, and command execution are recorded and surfaced rather
than silently ignored. Only push a PID into `killed` when the kill command
completes successfully with a successful exit status; preserve the existing
matching behavior while ensuring the final cleanup report distinguishes failures
from successful termination.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds an optional embedded net-kernel firmware variant while reusing the custom-kernel path already provided by #1041/#1051.
Test plan:
cargo fmt --all -- --checkDRY_RUN=1 make libkrunfw-netcargo build -p boxlite-shimkernel-net, custom-kernel RC, spawn selection)gvproxy_port_conflictx2 and invalid-command shim cleanup)Summary by CodeRabbit
New Features
--kernel-variant lean|netCLI option for selecting the runtime kernel.Bug Fixes
Documentation